refactor!: change model to output mel-band oriented tensors instead of time-oriented ones #94
Conversation
|
Review changes with Changed Files
|
| assert "output" in outputs and outputs["output"] is not None | ||
| assert wavs.shape[0] == outputs["output"].size( | ||
| wavs.ndim == 3 | ||
| ), f"The generated audio did not contain 3 dimensions. First dimension should be B(atch) and the second dimension should be C(hannels) and third dimension should be T(ime) in samples. Got {wavs.shape} instead." |
There was a problem hiding this comment.
Can this happen due to a user error (like providing the wrong kind of input file), or is this strictly due to a programmer error? If the latter, OK, if the former, I don't like using assert.
| basename=basename, | ||
| speaker=speaker, | ||
| language=language, | ||
| torchaudio.save( |
There was a problem hiding this comment.
Is the change of audio writer function related to this PR, or just an unrelated improvement? I assume you've tested and you can confirm this works well?
fs2/prediction_writing_callback.py
Outdated
| data[:unmasked_len] | ||
| .cpu() | ||
| .transpose(0, 1), # save tensors as [K (bands), T (frames)] | ||
| str( |
There was a problem hiding this comment.
We didn't use to need to cast this Path to a str, I wonder why you do now. In my PR #102, get_filename is factored out to the base class, we should have it do return str(path) in one place, in get_filename, instead of casting everywhere we use it.
Warning: Whichever PR is merged second will have to rebase and resolve conflicts over the use of get_filename.
There was a problem hiding this comment.
rebased and fixed using my suggestion here
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #94 +/- ##
==========================================
- Coverage 46.24% 46.13% -0.12%
==========================================
Files 22 22
Lines 1464 1461 -3
==========================================
- Hits 677 674 -3
Misses 787 787 ☔ View full report in Codecov by Sentry. |
See EveryVoiceTTS/EveryVoice#572